2 - Block components and hoc (PIWOO-761) #1084
Conversation
…anually when the order is in the pending payment state
…king. Also added fallback to search in meta-data when order is not found by transaction id
…nchronize-wc-order-status-with-mollie-payment-status Add Order Action to Synchronize WC Order Status with Mollie Payment Status
…nes-php-when-processing-vouchers-with-germanized-plugin TypeError in OrderLines.php when processing vouchers with Germanized plugin
Modernize webhooks with rest endpoint
Add an order not for payment details of Voucher payments
mmaymo
changed the title
|
|
||
| return ( | ||
| <div className="custom-input"> | ||
| <label htmlFor="billing-birthdate">{ label }</label> |
There was a problem hiding this comment.
The jsx tag isn't closed properly here, right? Also this can probably be a short tag
There was a problem hiding this comment.
Hmm, no, I think is closed right, is not a void element, but I will take a look at all the other elements around
| @@ -0,0 +1,19 @@ | |||
| export const BirthdateField = ( { label, value, onChange } ) => { | |||
| const handleChange = ( e ) => onChange( e.target.value ); | |||
There was a problem hiding this comment.
I think it would be an optimization to declare this function outside of the component (so it does not get reinitialized everytime we re-render)
As an alternative, perhaps look into useCallback https://react.dev/reference/react/useCallback
There was a problem hiding this comment.
I checked some docs, cause makes sense what you say, but if I understood right, declaring the function outside or using useCallback only helps if we pass it down to children or use it as a dependency in an effect. In our case the handler is only attached to a native (not a React child), so its identity doesn’t cause any extra renders. And if taking out I would need a way to pass the prop to that function. But I will add a new task to check in the parent components if the child components can be memoized.
| export const Label = ( { item } ) => { | ||
| return ( | ||
| <> | ||
| <div dangerouslySetInnerHTML={ { __html: item.label } } /> |
There was a problem hiding this comment.
I assume we are expecting markup within item.label ? Otherwise I wonder why we aren't simply passing it into the content
There was a problem hiding this comment.
yes, but there is no need to pass markup really, now we pass only data I've updated it
| const { useEffect } = wp.element; | ||
| const { useSelect } = wp.data; |
There was a problem hiding this comment.
I think these should be imports - at the very least they should not be assigned in the render function
There was a problem hiding this comment.
Right, I will tackle this in another PR
| billing_birthdate: inputBirthdate, | ||
| cardToken: '', | ||
| }; | ||
| const tokenVal = jQuery( '.mollie-components > input' ).val(); |
There was a problem hiding this comment.
If this is all we need jQuery for, should we not think about doing it in vanilla JS instead?
A more thorough solution would be to expose this type of data through te redux store and then consume it via withSelect and/or a higher-order-component
There was a problem hiding this comment.
Right, using the token from the store is done in a later PR but I've also expanded on the use of the store there after your suggestion, I think I can also use in the payment fields, I will update those as well. Removing jQuery is also a job started later but I will take a look in case i missed somewhere. Thanks!
Add React component architecture for Mollie checkout blocks
This PR is based on PIWOO-761-redux-store
(Some logs might still be there, will remove in the last round)